[MenuItem][Base] Pass idGenerator function #37364

sai6855 · 2023-05-22T13:47:13Z

(open menu in before sandbox to see the error)

before: https://codesandbox.io/s/falling-hooks-4tbj3g?file=/package.json
after: https://codesandbox.io/s/serverless-morning-xf15xb?file=/index.tsx

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2023-05-22T13:50:19Z

Netlify deploy preview

https://deploy-preview-37364--material-ui.netlify.app/

Bundle size report

Details of bundle changes

Generated by 🚫 dangerJS against 9f92907

sai6855 · 2023-05-23T09:04:45Z

packages/mui-base/src/utils/useCompound.ts

+        );
+      }
+      if (typeof id === 'function') {
+        providedOrGeneratedId = (id as MissingKeyGenerator<Key>)(subitemKeys.current);


facing below typescript error, if type of id is not casted to MissingKeyGenerator<Key>. Wasn't sure how to fix it. so went with casting approach.

Technically TS is correct because Key can be a function type. But I think we can leave it as is. I don't expect anyone to create a key that's a function.

sai6855 · 2023-05-23T09:06:33Z

packages/mui-base/src/utils/useCompoundItem.ts

@@ -55,13 +57,13 @@ export function useCompoundItem<Key, Subitem>(
  }

  const { registerItem } = context;
-  const [registeredId, setRegisteredId] = React.useState(id);
+  const [registeredId, setRegisteredId] = React.useState(typeof id === 'function' ? undefined : id);


since parameter to pass to the id function is not available at this point, i initiated value of registeredId to undefined if id is function.

sai6855 · 2023-05-23T09:09:48Z

packages/mui-base/src/useMenuItem/useMenuItem.ts

@@ -11,6 +11,10 @@ import {
 import { useListItem } from '../useList';
 import { useCompoundItem } from '../utils/useCompoundItem';

+function idGenerator(existingKeys: Set<string>) {


Not really sure how to test that if this function is being called or not as this function runs only on react versions less than 18.

michaldudak · 2023-05-23T13:30:11Z

packages/mui-base/src/utils/useCompound.ts

@@ -12,22 +12,22 @@ interface RegisterItemReturnValue<Key> {
  deregister: () => void;
 }

+export type MissingKeyGenerator<Key> = (existingKeys: Set<Key>) => Key;


Let's just call it KeyGenerator or KeyProvider

michaldudak · 2023-05-23T13:30:54Z

packages/mui-base/src/utils/useCompound.ts

   */
  registerItem: (
-    id: Key | undefined,
+    id: Key | MissingKeyGenerator<Key> | undefined,


I think undefined is no longer a valid option

michaldudak · 2023-05-23T13:32:38Z

packages/mui-base/src/utils/useCompound.ts

+        );
+      }
+      if (typeof id === 'function') {
+        providedOrGeneratedId = (id as MissingKeyGenerator<Key>)(subitemKeys.current);


Technically TS is correct because Key can be a function type. But I think we can leave it as is. I don't expect anyone to create a key that's a function.

michaldudak · 2023-05-23T13:33:28Z

packages/mui-base/src/utils/useCompoundItem.ts

@@ -32,18 +36,16 @@ export interface UseCompoundItemReturnValue<Key> {
 * @ignore - internal hook.
 */
 export function useCompoundItem<Key, Subitem>(
-  id: Key | undefined,
+  id: Key | MissingKeyGenerator<Key> | undefined,


As above, I think undefined can be dropped from here.

sai6855 · 2023-05-23T13:54:37Z

packages/mui-base/src/utils/useCompound.ts

      let providedOrGeneratedId: Key;
-      if (id === undefined) {
-        if (missingKeyGenerator === undefined) {
-          throw new Error(


since id cannot be undefined, i don't think throwing error makes much sense

sai6855 · 2023-05-23T14:12:41Z

@michaldudak done with all changes

michaldudak

👍 Looks good!

This comment was marked as outdated.

Sign in to view

sai6855 added component: menu This is the name of the generic UI component, not the React module! package: base-ui Specific to @mui/base regression A bug, but worse labels May 22, 2023

sai6855 mentioned this pull request May 22, 2023

[MenuItem][Joy] MenuItem breaks in React 17 since release v5.12.1 #37251

Closed

sai6855 marked this pull request as draft May 22, 2023 14:24

sai6855 changed the title ~~[MenuItem][Base] Pass missingKeyGenerator to useCompoundItem~~ [MenuItem][Base][Joy] Add missingKeyGenerator prop to MenuItem May 22, 2023

sai6855 mentioned this pull request May 22, 2023

[MenuItem][Joy] Add internal id to DOM #37361

Closed

1 task

sai6855 force-pushed the joy-menu-alternate branch from fbb1954 to 630f718 Compare May 23, 2023 07:19

[MenuItem][Base] Pass idGenerator

bcdfd0b

sai6855 force-pushed the joy-menu-alternate branch from 630f718 to bcdfd0b Compare May 23, 2023 08:07

sai6855 changed the title ~~[MenuItem][Base][Joy] Add missingKeyGenerator prop to MenuItem~~ [MenuItem][Base] Pass idGenerator function May 23, 2023

sai6855 mentioned this pull request May 23, 2023

[core] Return valid id on first render of useId hook #37333

Closed

1 task

fix-tests

0758c45

sai6855 commented May 23, 2023

View reviewed changes

sai6855 marked this pull request as ready for review May 23, 2023 09:10

sai6855 requested a review from michaldudak May 23, 2023 09:16

michaldudak reviewed May 23, 2023

View reviewed changes

sai6855 added 2 commits May 23, 2023 13:45

update changes

b90ae7b

remove error

3583dbd

sai6855 commented May 23, 2023

View reviewed changes

michaldudak approved these changes May 24, 2023

View reviewed changes

Merge branch 'master' into joy-menu-alternate

9f92907

sai6855 requested a review from michaldudak May 24, 2023 13:33

michaldudak approved these changes May 24, 2023

View reviewed changes

hbjORbj merged commit f3f93a3 into mui:master May 29, 2023

oliviertassinari mentioned this pull request Jan 28, 2024

[core] missingKeyGenerator do no longer exist #40830

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MenuItem][Base] Pass idGenerator function #37364

[MenuItem][Base] Pass idGenerator function #37364

sai6855 commented May 22, 2023 •

edited

Loading

This comment was marked as outdated.

mui-bot commented May 22, 2023 •

edited

Loading

sai6855 May 23, 2023 •

edited

Loading

michaldudak May 23, 2023

sai6855 May 23, 2023

sai6855 May 23, 2023 •

edited

Loading

michaldudak May 23, 2023

michaldudak May 23, 2023

michaldudak May 23, 2023

michaldudak May 23, 2023

sai6855 May 23, 2023

sai6855 commented May 23, 2023

michaldudak left a comment

[MenuItem][Base] Pass idGenerator function #37364

[MenuItem][Base] Pass idGenerator function #37364

Conversation

sai6855 commented May 22, 2023 • edited Loading

This comment was marked as outdated.

mui-bot commented May 22, 2023 • edited Loading

Netlify deploy preview

Bundle size report

sai6855 May 23, 2023 • edited Loading

Choose a reason for hiding this comment

michaldudak May 23, 2023

Choose a reason for hiding this comment

sai6855 May 23, 2023

Choose a reason for hiding this comment

sai6855 May 23, 2023 • edited Loading

Choose a reason for hiding this comment

michaldudak May 23, 2023

Choose a reason for hiding this comment

michaldudak May 23, 2023

Choose a reason for hiding this comment

michaldudak May 23, 2023

Choose a reason for hiding this comment

michaldudak May 23, 2023

Choose a reason for hiding this comment

sai6855 May 23, 2023

Choose a reason for hiding this comment

sai6855 commented May 23, 2023

michaldudak left a comment

Choose a reason for hiding this comment

sai6855 commented May 22, 2023 •

edited

Loading

mui-bot commented May 22, 2023 •

edited

Loading

sai6855 May 23, 2023 •

edited

Loading

sai6855 May 23, 2023 •

edited

Loading